Conversation
- Updated the Makefile to include test commands for the indexer. - Modified integration test configuration to use specific database user and name. - Refactored `ProcessValidatorSignings` to handle commits instead of blocks, updating related methods and tests accordingly more detail in the CHANGELOG once all has been implemented. - Implemented new methods for retrieving commits in the query operator. - Added to synthetic query operator a function to generate and return synthetic commits for testing. - Improved error handling and logging for commit retrieval. - Updated tests to cover new commit functionality and ensure proper behavior.
- Added a `message_counter` field to message types (MsgSend, MsgCall, MsgAddPackage, MsgRun) to track message order and have unique message counter. - Updated the `GetMessageFromStdTx` method to include `message_counter` in the processed messages. - Modified error handling in the database insertion methods to log transaction hashes on failure. - Refactored the `Orchestrator` to streamline chunk processing and improve concurrency in block and commit retrieval.
- Updated all database interaction methods to accept a context parameter, allowing for better control over timeouts and cancellation.
- Modified related methods in the address cache and data processor to utilize the new context-aware database methods.
- Data processor now processes the data using map[string]struct{} and sync mux.
- Database queries should now return the message counter for the api queries.
- Updated API handlers to pass context to database methods. - Refactored transaction handling to support multiple message types within a single transaction, including adjustments to the response structure. - Added favicon support to the API. - Improved OpenAPI documentation with detailed descriptions for various endpoints.
Stage dev Release/0.4.0
WalkthroughThreads context.Context across DB/RPC layers, adds per-message message_counter and DB/schema fields, introduces commit RPCs and commit-based processing alongside blocks, refactors orchestrator/data-processor flows for chunked parallel fetches, and updates OpenAPI, tooling, tests, and workflows. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as HTTP Client
participant Handler as API Handler
participant DB as TimescaleDb
participant Ctx as Context
Client->>Handler: HTTP request
Handler->>Ctx: create ctx (with timeout)
Handler->>DB: GetMsgTypes(ctx, txHash, chain)
DB-->>Handler: []string (message types)
loop per message type
Handler->>DB: Get<MsgX>(ctx, txHash, chain) -> []*MsgX (each has MessageCounter)
DB-->>Handler: messages
Handler->>Handler: place messages into map[int16]Response
end
Handler-->>Client: multi-message response (map[int16]TransactionMessage)
sequenceDiagram
participant Orch as Orchestrator
participant QO as QueryOperator
participant RPC as RpcClient
participant DP as DataProcessor
Orch->>QO: GetFromToBlocks(from,to)
Orch->>QO: GetFromToCommits(from,to) [NEW]
par parallel fetch
QO->>RPC: GetBlock(height) x N
QO->>RPC: GetCommit(height) x N [NEW]
end
QO-->>Orch: blocks, commits
Orch->>DP: ProcessValidatorSignings(commits, from,to) [CHANGED]
DP-->>Orch: processed
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Areas to focus review on:
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
indexer/db_init/hypertable.go (1)
90-90: Fix typo in AddCompressionPolicy documentation.Line 90 has "TThis" which should be "This".
-// TThis function specifies the compression policy +// This function specifies the compression policy.github/workflows/integration-tests.yml (1)
60-60: I'll help you verify the consistency of the database setup commands across the three integration test jobs. Let me start by examining the workflow file to understand the full context.
<function_calls>
cat -n .github/workflows/integration-tests.yml | head -250
</function_calls>
sed -n '50,70p' .github/workflows/integration-tests.yml
</function_calls>
sed -n '125,145p' .github/workflows/integration-tests.yml
</function_calls>
sed -n '200,215p' .github/workflows/integration-tests.yml
</function_calls>Now let me check the indexer command to understand what the flags do and whether they have defaults:
<function_calls>
find . -name "main.go" -path "/indexer/" | head -5
</function_calls>
<function_calls>
rg -n "create-db" --type=go -A 10 -B 5
</function_calls>Let me get more information about the command definition:
<function_calls>
rg -n "db-user|db-name" --type=go -B 5 -A 2 | head -100
</function_calls>Let me search more broadly for the setup command definition:
<function_calls>
rg -n "setup" --type=go | grep -i "command|flag" | head -20
</function_calls>Let me look at the environment variables defined in the workflow:
<function_calls>
sed -n '1,100p' .github/workflows/integration-tests.yml | grep -A 20 "env:"
</function_calls><function_calls>
sed -n '50,100p' .github/workflows/integration-tests.yml
</function_calls>Let me get a comprehensive view of the entire workflow file structure:
<function_calls>
wc -l .github/workflows/integration-tests.yml
</function_calls><function_calls>
cat .github/workflows/integration-tests.yml
</function_calls>Perfect! Now let me search for the command implementation to understand the flag behavior:
<function_calls>
find . -type f -name "*.go" | xargs grep -l "create-db" | head -5
</function_calls><function_calls>
rg -n "create-db|createDB" --type=go -B 3 -A 15 | head -150
</function_calls>Let me search more specifically:
<function_calls>
rg -n "flag.(String|Bool|Int)" --type=go | grep -E "(db.user|db.name|db_user|db_name)" | head -20
</function_calls><function_calls>
fd "main.go" --path "indexer" -x cat {}
</function_calls>Let me try a different approach to find where flags are defined:
<function_calls>
rg -n "db-user|db-name" --type=go | head -30
</function_calls><function_calls>
rg -n "dbUser|dbName" --type=go -B 2 -A 2 | head -50
</function_calls>Let me check the repository structure:
<function_calls>
git ls-files | grep -E "indexer|cmd" | head -20
</function_calls><function_calls>
git ls-files "indexer/" "cmd/" | head -30
</function_calls><function_calls>
ls -la indexer/ 2>/dev/null || ls -la cmd/ 2>/dev/null || find . -maxdepth 2 -type d -name "indexer" -o -name "cmd" 2>/dev/null
</function_calls>Let me check what files are actually in the repo:
<function_calls>
git ls-files | head -50
</function_calls>Now let me search for any references to the setup command:
<function_calls>
rg -n "setup" --type=go -B 2 -A 8 | head -200
</function_calls><function_calls>
rg -n "postgres" --type=go | head -40
</function_calls>Let me check the environment variables more carefully in all three jobs:
<function_calls>
grep -n "db-user|db-name|DB_USER|DB_NAME" .github/workflows/integration-tests.yml
</function_calls><function_calls>
python3 << 'EOF'
import rewith open('.github/workflows/integration-tests.yml', 'r') as f:
content = f.read()Find all test job sections
jobs = re.findall(r'integration-tests-(small|medium|large):(.*?)(?=\n \w+-tests-\w+:|$)', content, re.DOTALL)
for job_name, job_content in jobs:
# Find the create-db command
cmd_match = re.search(r'go run indexer/main.go setup create-db[^\n]*', job_content)
if cmd_match:
print(f"\n=== {job_name.upper()} ===")
print(f"Command: {cmd_match.group(0)}")# Find env vars env_match = re.search(r'env:(.*?)(?=\n\s{6}\w+:|$)', job_content, re.DOTALL) if env_match: print(f"Environment variables:") for line in env_match.group(1).strip().split('\n'): if line.strip(): print(f" {line.strip()}")EOF
</function_calls>Perfect! Now I have clear visibility. Let me verify if there's any documentation about default values for these flags:
<function_calls>
rg -n "DB_USER|DB_NAME" --type=go -B 5 -A 5 | head -100
</function_calls><function_calls>
rg -n "getenv|os." --type=go | grep -i "db_user|db_name" | head -20
</function_calls>Let me search for any flag defaults or initial values:
<function_calls>
rg -n "NewCommand|NewRunner" --type=go -A 20 | head -100
</function_calls>Let me check git history to see if this was intentional or a mistake:
<function_calls>
git log --oneline -20 -- .github/workflows/integration-tests.yml
</function_calls><function_calls>
git log -p --max-count=5 -- .github/workflows/integration-tests.yml | head -300
</function_calls>Let me check if there are any recent changes to the large test job specifically:
<function_calls>
git show HEAD:.github/workflows/integration-tests.yml | sed -n '200,215p'
</function_calls>Based on my analysis of the workflow file, I can confirm the inconsistency exists as stated in the original review comment:
- Line 60 (small test):
go run indexer/main.go setup create-db --db-user postgres --db-name postgres- Line 134 (medium test):
go run indexer/main.go setup create-db --db-user postgres --db-name postgres- Line 208 (large test):
go run indexer/main.go setup create-dbAll three jobs define
DB_USER: postgresin their environment variables, but the small and medium tests additionally pass the--db-user postgres --db-name postgresflags explicitly, while the large test omits them.While I cannot access the Go implementation details in this sandbox environment to determine whether the flags are truly redundant or necessary, the clear inconsistency across test sizes without any code comments explaining the difference suggests this is unintentional. The pattern of flags being included in two places but omitted in the third is characteristic of a configuration inconsistency that would benefit from clarification or correction.
Database setup command flags should be consistent across all integration test sizes.
The small and medium test jobs include
--db-user postgres --db-name postgresflags, while the large test job omits them despite setting the sameDB_USER: postgresenvironment variable. This inconsistency could indicate either:
- An unintentional omission in the large test that should be corrected to match the others
- A deliberate difference that should be documented
Verify whether the large test should include the same flags:
- name: Initialize database schema run: | - go run indexer/main.go setup create-db + go run indexer/main.go setup create-db --db-user postgres --db-name postgres env:If the flags are truly redundant with environment variables, remove them consistently from all three test jobs for clarity and maintainability.
experiments/experiment1/enc_dec_test.go (1)
42-43: Add newline characters to Printf statements.Both Printf statements are missing newline characters (
\n), causing the output to be printed on the same line, which makes it difficult to read.Apply this diff to fix the formatting:
- fmt.Printf("The final base64 decoded has %v bytes", len(rawByte)) - fmt.Printf("The first decoded base64 has %v bytes", len(base64Decoded)) + fmt.Printf("The final base64 decoded has %v bytes\n", len(rawByte)) + fmt.Printf("The first decoded base64 has %v bytes\n", len(base64Decoded))pkgs/generator/generator_test.go (1)
41-50: Fix panic recovery that hides test failures.The panic recovery logs the error but doesn't fail the test with
t.Fatal()ort.Error(). This means panics will be hidden and the test will incorrectly pass.// RunAllTests runs all the test functions func RunAllTests(t *testing.T) { defer func() { if r := recover(); r != nil { - log.Printf("Test failed with panic: %v", r) + t.Fatalf("Test failed with panic: %v", r) } }() TestAuthenticGeneration(t) }indexer/decoder/decoder.go (1)
214-223: Bug:extractCoinsproduces duplicate entries.The function pre-allocates a slice with
make([]Coin, len(amount))(filled with zero values), then usesappendwhich adds elements after the zero values, resulting in duplicate/empty entries.Apply this diff to fix:
func extractCoins(amount std.Coins) ([]Coin, error) { - // make a string and split it by space - coins := make([]Coin, len(amount)) - for _, coin := range amount { - coins = append(coins, Coin{ + coins := make([]Coin, 0, len(amount)) + for _, coin := range amount { + coins = append(coins, Coin{ Amount: coin.Amount, Denom: coin.Denom, }) } return coins, nil }Alternatively, use index-based assignment:
func extractCoins(amount std.Coins) ([]Coin, error) { - // make a string and split it by space coins := make([]Coin, len(amount)) - for _, coin := range amount { - coins = append(coins, Coin{ + for i, coin := range amount { + coins[i] = Coin{ Amount: coin.Amount, Denom: coin.Denom, - }) + } } return coins, nil }pkgs/database/queries_api.go (2)
146-156: Missingrows.Err()check in GetFromToBlocks.After iterating through rows, you should check for any errors that occurred during iteration. This is done correctly in
GetLastXBlocks(line 108) but missing here.for rows.Next() { block := &BlockData{} err := rows.Scan(&block.Hash, &block.Height, &block.Timestamp, &block.ChainID, &block.Txs) if err != nil { return nil, err } blocks = append(blocks, block) } + if err := rows.Err(); err != nil { + return nil, err + } return blocks, nil
212-260: GetBankSend is missingmessage_counterin the query.The query selects columns for BankSend but omits
bms.message_counter, while other similar methods (GetMsgCall, GetMsgAddPackage, GetMsgRun) include it. TheBankSendstruct has aMessageCounterfield (per types_api.go), so this field will always be zero in API responses.query := ` SELECT + bms.message_counter, encode(bms.tx_hash, 'base64') AS tx_hash, bms.timestamp, gn_from.address AS from_address, gn_to.address AS to_address, bms.amount, array( SELECT gn.address FROM unnest(bms.signers) AS signer_id JOIN gno_addresses gn ON gn.id = signer_id ) AS signers FROM bank_msg_send bmsAnd update the Scan call to include
&bankSend.MessageCounteras the first field:err := rows.Scan( + &bankSend.MessageCounter, &bankSend.TxHash, &bankSend.Timestamp,
🧹 Nitpick comments (19)
Makefile (1)
1-1: Consider adding the "all" phony target.Best practice for Makefiles is to include an
alltarget as the default goal. While your current targets work, addingallas the first PHONY target and defining it as a prerequisite to build, build-api, and build-experimental would improve discoverability and conventionality.-.PHONY: build install clean build-experimental install-experimental build-api test-race integration-test test +.PHONY: all build install clean build-experimental install-experimental build-api test-race integration-test test + +all: build build-apiCHANGELOG.md (2)
10-10: Consider more formal language for release notes.The opening line "Mostly it has some bug fixes" uses informal phrasing. Consider revising to something like: "This release includes several bug fixes and improvements."
10-10: Consider more formal language for release notes opening.The phrasing "Mostly it has some bug fixes" is informal for release notes. Consider revising to: "This release includes several bug fixes and improvements."
experiments/experiment1/enc_dec_test.go (1)
18-18: Consider using idiomatic Go naming convention.The variable
raw_datauses snake_case instead of the idiomatic Go camelCase convention. Consider renaming it torawDatafor consistency with Go style guidelines.Apply this diff to improve naming:
- raw_data := "CpABCgovdm0ubV9jYWxsEoEBCihnMTcyOTBjd3ZtcmFwdnA4Njl4Zm5oaGF3YThzbTllZHB1ZnphdDdkIhZnbm8ubGFuZC9yL2dub3N3YXAvZ25zKghUcmFuc2ZlcjIoZzEyejc3bHk0aHdyeWtqMnphZmNoZmR2bGxmZm5wbDU1d2dzcHVjbDIJMTAwMDAwMDAwCpwBCgovdm0ubV9jYWxsEo0BCihnMTcyOTBjd3ZtcmFwdnA4Njl4Zm5oaGF3YThzbTllZHB1ZnphdDdkIiJnbm8ubGFuZC9yL2dub3N3YXAvdGVzdF90b2tlbi91c2RjKghUcmFuc2ZlcjIoZzEyejc3bHk0aHdyeWtqMnphZmNoZmR2bGxmZm5wbDU1d2dzcHVjbDIJMTAwMDAwMDAwCpsBCgovdm0ubV9jYWxsEowBCihnMTcyOTBjd3ZtcmFwdnA4Njl4Zm5oaGF3YThzbTllZHB1ZnphdDdkIiFnbm8ubGFuZC9yL2dub3N3YXAvdGVzdF90b2tlbi9iYXIqCFRyYW5zZmVyMihnMTJ6NzdseTRod3J5a2oyemFmY2hmZHZsbGZmbnBsNTV3Z3NwdWNsMgkxMDAwMDAwMDAKmwEKCi92bS5tX2NhbGwSjAEKKGcxNzI5MGN3dm1yYXB2cDg2OXhmbmhoYXdhOHNtOWVkcHVmemF0N2QiIWduby5sYW5kL3IvZ25vc3dhcC90ZXN0X3Rva2VuL2JheioIVHJhbnNmZXIyKGcxMno3N2x5NGh3cnlrajJ6YWZjaGZkdmxsZmZucGw1NXdnc3B1Y2wyCTEwMDAwMDAwMAqbAQoKL3ZtLm1fY2FsbBKMAQooZzE3MjkwY3d2bXJhcHZwODY5eGZuaGhhd2E4c205ZWRwdWZ6YXQ3ZCIhZ25vLmxhbmQvci9nbm9zd2FwL3Rlc3RfdG9rZW4vb2JsKghUcmFuc2ZlcjIoZzEyejc3bHk0aHdyeWtqMnphZmNoZmR2bGxmZm5wbDU1d2dzcHVjbDIJMTAwMDAwMDAwCpsBCgovdm0ubV9jYWxsEowBCihnMTcyOTBjd3ZtcmFwdnA4Njl4Zm5oaGF3YThzbTllZHB1ZnphdDdkIiFnbm8ubGFuZC9yL2dub3N3YXAvdGVzdF90b2tlbi9mb28qCFRyYW5zZmVyMihnMTJ6NzdseTRod3J5a2oyemFmY2hmZHZsbGZmbnBsNTV3Z3NwdWNsMgkxMDAwMDAwMDAKmwEKCi92bS5tX2NhbGwSjAEKKGcxNzI5MGN3dm1yYXB2cDg2OXhmbmhoYXdhOHNtOWVkcHVmemF0N2QiIWduby5sYW5kL3IvZ25vc3dhcC90ZXN0X3Rva2VuL3F1eCoIVHJhbnNmZXIyKGcxMno3N2x5NGh3cnlrajJ6YWZjaGZkdmxsZmZucGw1NXdnc3B1Y2wyCTEwMDAwMDAwMBITCICEr18SDDEwMDAwMDB1Z25vdBp+CjoKEy90bS5QdWJLZXlTZWNwMjU2azESIwohAtGyA6l3UIrUup5z7yXo90bXDcXUOmLiK34YPffgQ6pAEkCg8pAtBv6Fhw98bKYdqrEX2UrcjYvIUpbGdxMyc5Zpfl1cMNo8G6vzpnFaQESX+7eZIFTfO5BCFjVJLfC5Ur5C" + rawData := "CpABCgovdm0ubV9jYWxsEoEBCihnMTcyOTBjd3ZtcmFwdnA4Njl4Zm5oaGF3YThzbTllZHB1ZnphdDdkIhZnbm8ubGFuZC9yL2dub3N3YXAvZ25zKghUcmFuc2ZlcjIoZzEyejc3bHk0aHdyeWtqMnphZmNoZmR2bGxmZm5wbDU1d2dzcHVjbDIJMTAwMDAwMDAwCpwBCgovdm0ubV9jYWxsEo0BCihnMTcyOTBjd3ZtcmFwdnA4Njl4Zm5oaGF3YThzbTllZHB1ZnphdDdkIiJnbm8ubGFuZC9yL2dub3N3YXAvdGVzdF90b2tlbi91c2RjKghUcmFuc2ZlcjIoZzEyejc3bHk0aHdyeWtqMnphZmNoZmR2bGxmZm5wbDU1d2dzcHVjbDIJMTAwMDAwMDAwCpsBCgovdm0ubV9jYWxsEowBCihnMTcyOTBjd3ZtcmFwdnA4Njl4Zm5oaGF3YThzbTllZHB1ZnphdDdkIiFnbm8ubGFuZC9yL2dub3N3YXAvdGVzdF90b2tlbi9iYXIqCFRyYW5zZmVyMihnMTJ6NzdseTRod3J5a2oyemFmY2hmZHZsbGZmbnBsNTV3Z3NwdWNsMgkxMDAwMDAwMDAKmwEKCi92bS5tX2NhbGwSjAEKKGcxNzI5MGN3dm1yYXB2cDg2OXhmbmhoYXdhOHNtOWVkcHVmemF0N2QiIWduby5sYW5kL3IvZ25vc3dhcC90ZXN0X3Rva2VuL2JheioIVHJhbnNmZXIyKGcxMno3N2x5NGh3cnlrajJ6YWZjaGZkdmxsZmZucGw1NXdnc3B1Y2wyCTEwMDAwMDAwMAqbAQoKL3ZtLm1fY2FsbBKMAQooZzE3MjkwY3d2bXJhcHZwODY5eGZuaGhhd2E4c205ZWRwdWZ6YXQ3ZCIhZ25vLmxhbmQvci9nbm9zd2FwL3Rlc3RfdG9rZW4vb2JsKghUcmFuc2ZlcjIoZzEyejc3bHk0aHdyeWtqMnphZmNoZmR2bGxmZm5wbDU1d2dzcHVjbDIJMTAwMDAwMDAwCpsBCgovdm0ubV9jYWxsEowBCihnMTcyOTBjd3ZtcmFwdnA4Njl4Zm5oaGF3YThzbTllZHB1ZnphdDdkIiFnbm8ubGFuZC9yL2dub3N3YXAvdGVzdF90b2tlbi9mb28qCFRyYW5zZmVyMihnMTJ6NzdseTRod3J5a2oyemFmY2hmZHZsbGZmbnBsNTV3Z3NwdWNsMgkxMDAwMDAwMDAKmwEKCi92bS5tX2NhbGwSjAEKKGcxNzI5MGN3dm1yYXB2cDg2OXhmbmhoYXdhOHNtOWVkcHVmemF0N2QiIWduby5sYW5kL3IvZ25vc3dhcC90ZXN0X3Rva2VuL3F1eCoIVHJhbnNmZXIyKGcxMno3N2x5NGh3cnlrajJ6YWZjaGZkdmxsZmZucGw1NXdnc3B1Y2wyCTEwMDAwMDAwMBITCICEr18SDDEwMDAwMDB1Z25vdBp+CjoKEy90bS5QdWJLZXlTZWNwMjU2azESIwohAtGyA6l3UIrUup5z7yXo90bXDcXUOmLiK34YPffgQ6pAEkCg8pAtBv6Fhw98bKYdqrEX2UrcjYvIUpbGdxMyc5Zpfl1cMNo8G6vzpnFaQESX+7eZIFTfO5BCFjVJLfC5Ur5C"And update the reference on line 21:
- base64Decoded, err := base64.StdEncoding.DecodeString(raw_data) + base64Decoded, err := base64.StdEncoding.DecodeString(rawData)experiments/experiment4/encode_proto.go (1)
4-4: Per‑level compression loop looks correct; consider minor ergonomic cleanupsThe per‑level benchmarking loop is sane: fresh
bytes.Bufferper iteration,encoder.Reset(&buf), write, close, and then inspectbuf.Bytes()is all correct. The newbytesimport is appropriate.If you want to tighten it up a bit:
- You can construct the writer directly with the buffer instead of
NewWriter(nil)+Reset(&buf), e.g.zstandard.NewWriter(&buf, zstandard.WithEncoderLevel(encoderLevel)), which avoids one extra call.- The “encode into buffer and return compressed bytes” pattern is duplicated with the final level‑22 block; it could be extracted into a small helper like
compressAtLevel(encoded []byte, level int) ([]byte, error)to reduce repetition, but that’s optional given this lives underexperiments/.Also applies to: 86-107
indexer/context_hook/hook.go (1)
35-41: Minor documentation inconsistency.The
Parameterssection listssh: the signal handler, butshis a method receiver, not a parameter. Consider removing this entry or clarifying it as "Receiver" for accuracy.// Context returns the context that will be cancelled on shutdown signals // -// Parameters: -// - sh: the signal handler -// // Returns: // - context.Context: the context //indexer/query/types.go (1)
24-36: Update interface doc comment to includeGetCommit.The doc comment lists
GetBlock,GetLatestBlockHeight, andGetTxbut omits the newly addedGetCommitmethod.// Rate limiter Gnoland RPC client interface // // # The interface is used to get the blocks, latest block height, and txs // // Methods: // - GetBlock: to get a block from the rpc client // - GetLatestBlockHeight: to get the latest block height from the rpc client // - GetTx: to get a tx from the rpc client +// - GetCommit: to get a commit from the rpc client type RpcClient interface {indexer/rpc_client/errors.go (1)
35-48: Consider consolidating duplicate error types.
RpcCommitErroris structurally identical toRpcHeightError(same fields, sameError()implementation). If semantic type safety isn't strictly required, you could use a single generic or aliased type to reduce duplication.However, if distinct types are intentional for call-site clarity (e.g., differentiating commit errors from block errors in type switches), the current approach is acceptable.
Example consolidation using a type alias:
// If semantic distinction isn't critical at call sites: type RpcCommitError = RpcHeightErrorindexer/decoder/product.go (1)
263-270: Optional: Consider consistent field ordering in struct literals.
MessageCounteris placed last inMsgSendbut second (afterTxHash) in other message types. For consistency and easier comparison, consider aligning the field order across all struct literals.return &dataTypes.MsgSend{ + MessageCounter: messageCounter, TxHash: txHash, ChainName: chainName, FromAddress: addressResolver.GetAddress(fromAddress), ToAddress: addressResolver.GetAddress(toAddress), Amount: amount, Signers: signerIds, Timestamp: timestamp, - MessageCounter: messageCounter, }, nilindexer/cmd/setup.go (1)
100-102: Context timeout implementation is correct.The 10-second timeout context with proper
defer cancel()is well-implemented. However, note that onlyCheckCurrentDatabaseNamereceives the context while subsequent database operations (CreateDatabase,SwitchDatabase,CreateSpecialTypeFromStruct, etc.) do not. Consider propagating context to these operations as well for consistent cancellation behavior throughout the setup flow.indexer/query/operator.go (1)
143-201:GetFromToCommitscorrectly mirrorsGetFromToBlockspattern.The implementation follows the established concurrent fetch pattern with proper:
- Slice preallocation with exact size
- Mutex-protected slice access
- WaitGroup coordination
- Retry mechanism with success/failure callbacks
Consider extracting the common parallel-fetch-with-retry pattern into a generic helper to reduce duplication between
GetFromToBlocks,GetFromToCommits, andGetTransactions. For example:func parallelFetch[T any]( count int, fetch func(idx int) (T, error), retryFetch func(idx int) (T, error), retryConfig RetryConfig, ) []T { // shared implementation }api/handlers/transactions.go (1)
43-146: Consider renaming the loop variable to avoid shadowing.The variable
datais used both for the slice returned by the database query and as the loop iteration variable (lines 67, 84, 104, 124). While this is valid Go, it reduces clarity.Consider renaming the loop variable for improved readability:
data, err := h.db.GetBankSend(ctx, txHashBase64, h.chainName) if err != nil { return nil, huma.Error404NotFound(fmt.Sprintf("Failed to fetch %s data for transaction %s", msgType, input.TxHash), err) } -for _, data := range data { - index := data.MessageCounter +for _, msg := range data { + index := msg.MessageCounter response[index] = humatypes.TransactionMessage{ MessageType: msgType, - TxHash: data.TxHash, - Timestamp: data.Timestamp, - Signers: data.Signers, - FromAddress: data.FromAddress, - ToAddress: data.ToAddress, - Amount: data.Amount, + TxHash: msg.TxHash, + Timestamp: msg.Timestamp, + Signers: msg.Signers, + FromAddress: msg.FromAddress, + ToAddress: msg.ToAddress, + Amount: msg.Amount, } }Apply similar changes to the other message type cases (vm_msg_call, vm_msg_add_package, vm_msg_run).
integration/synthetic/query_operator.go (2)
100-113: Inconsistent nil check compared toGetFromToBlocks.
GetFromToBlocks(lines 68-82) checks if each block is non-nil before appending, butGetFromToCommitsunconditionally appends the result ofgetCommit. SincegetCommitcallslog.Fatalif not found, this is safe in practice, but the pattern differs. Consider unifying the approach or documenting the invariant.for height := fromHeight; height <= toHeight; height++ { - commits = append(commits, sq.getCommit(height)) + commit := sq.getCommit(height) + if commit != nil { + commits = append(commits, commit) + } }
240-252: Proposer selection is deterministic, not random as comment suggests.Line 246 uses modulo arithmetic
height%uint64(len(sq.signedValidators))which is deterministic based on height, not random. The comment says "random validator" which is misleading.- ProposerAddress: sq.signedValidators[height%uint64(len(sq.signedValidators))], // random validator + ProposerAddress: sq.signedValidators[height%uint64(len(sq.signedValidators))], // deterministic proposer based on heightindexer/address_cache/address.go (1)
115-120: Consider usingdefer cancel()to prevent context leak on early return.If
FindExistingAccountsreturns an error at line 119, the function returns immediately. Thecancel()at line 117 is called, but usingdefer cancel()is the idiomatic pattern and protects against future code changes that might add more exit paths.ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) + defer cancel() existingAddresses, err := a.db.FindExistingAccounts(ctx, address, chainName, insertValidators) - cancel() if err != nil { return }indexer/data_processor/operator.go (1)
62-98: Mutex-protected map is correct but consider reducing lock contention.The pattern of locking for each individual write (lines 78-80, 86-88) works correctly but may cause contention with many concurrent goroutines. For high-volume scenarios, consider using a sync.Map or collecting addresses locally per goroutine and merging after
wg.Wait().integration/synthetic/respone_maker.go (1)
1-1: Filename typo: "respone_maker.go" should be "response_maker.go".The filename has a typo - missing the 's' in "response".
Consider renaming the file from
respone_maker.gotoresponse_maker.gofor correctness.api/handlers/interface.go (1)
13-13: Consider using[]database.AddressTxinstead of*[]database.AddressTxfor consistency.The return type
*[]database.AddressTxis inconsistent with other methods in this interface that return slices directly (e.g.,[]*database.BlockData,[]*database.Transaction,[]*database.BankSend). In Go, slices are already reference types, so returning a pointer to a slice is rarely necessary and can lead to nil-pointer confusion.- GetAddressTxs(ctx context.Context, address string, chainName string, fromTimestamp time.Time, toTimestamp time.Time) (*[]database.AddressTx, error) + GetAddressTxs(ctx context.Context, address string, chainName string, fromTimestamp time.Time, toTimestamp time.Time) ([]database.AddressTx, error)pkgs/database/queries_api.go (1)
608-645: GetAddressTxs returns*[]AddressTx- consider returning[]AddressTxinstead.Consistent with the earlier observation on the interface, returning a pointer to a slice is unusual in Go. The method implementation already creates the slice as a value (
addressTxs := make([]AddressTx, 0)), then returns a pointer to it. This works but is inconsistent with other query methods that return[]*Type.func (t *TimescaleDb) GetAddressTxs( ctx context.Context, address string, chainName string, fromTimestamp time.Time, toTimestamp time.Time, -) (*[]AddressTx, error) { +) ([]AddressTx, error) { ... - return &addressTxs, nil + return addressTxs, nil
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
api/public/favicon.icois excluded by!**/*.ico
📒 Files selected for processing (52)
.github/workflows/golangci.yml(1 hunks).github/workflows/gotest.yml(1 hunks).github/workflows/integration-tests.yml(2 hunks)CHANGELOG.md(1 hunks)Makefile(2 hunks)api/handlers/address.go(1 hunks)api/handlers/blocks.go(5 hunks)api/handlers/database_test.go(10 hunks)api/handlers/interface.go(2 hunks)api/handlers/transactions.go(3 hunks)api/huma-types/transaction.go(2 hunks)api/main.go(4 hunks)experiments/experiment1/enc_dec_test.go(1 hunks)experiments/experiment4/encode_proto.go(2 hunks)go.mod(1 hunks)indexer/address_cache/address.go(9 hunks)indexer/address_cache/types.go(1 hunks)indexer/cmd/setup.go(2 hunks)indexer/context_hook/hook.go(2 hunks)indexer/data_processor/event.go(1 hunks)indexer/data_processor/operator.go(24 hunks)indexer/data_processor/operator_test.go(3 hunks)indexer/data_processor/types.go(2 hunks)indexer/db_init/hypertable.go(3 hunks)indexer/db_init/tables.go(10 hunks)indexer/decoder/decoder.go(8 hunks)indexer/decoder/product.go(9 hunks)indexer/main_operator/operator.go(2 hunks)indexer/orchestrator/operator.go(12 hunks)indexer/orchestrator/operator_test.go(8 hunks)indexer/orchestrator/types.go(2 hunks)indexer/query/operator.go(2 hunks)indexer/query/operator_test.go(1 hunks)indexer/query/types.go(1 hunks)indexer/retry/worker.go(1 hunks)indexer/rpc_client/client.go(7 hunks)indexer/rpc_client/commit.go(1 hunks)indexer/rpc_client/errors.go(1 hunks)indexer/rpc_client/rate_limit/rate_limit.go(1 hunks)indexer/rpc_client/rate_limited_client.go(3 hunks)indexer/rpc_client/types.go(1 hunks)integration/synthetic/init.go(2 hunks)integration/synthetic/query_operator.go(6 hunks)integration/synthetic/respone_maker.go(5 hunks)integration/test_config.example.yml(1 hunks)pkgs/database/insert.go(14 hunks)pkgs/database/pool.go(5 hunks)pkgs/database/queries_api.go(26 hunks)pkgs/database/queries_indexer.go(7 hunks)pkgs/database/types_api.go(1 hunks)pkgs/generator/generator_test.go(1 hunks)pkgs/sql_data_types/table.go(7 hunks)
🧰 Additional context used
🧬 Code graph analysis (24)
indexer/rpc_client/types.go (2)
indexer/rpc_client/commit.go (1)
CommitResponse(10-15)indexer/rpc_client/errors.go (1)
RpcCommitError(36-40)
pkgs/generator/generator_test.go (3)
pkgs/generator/general.go (5)
NewDataGenerator(43-54)rand(37-41)g(62-64)g(244-314)g(67-69)pkgs/generator/crypto.go (3)
ValidateAddress(95-104)ValidatePubKey(107-117)rand(12-14)experiments/experiment7/main.go (2)
testKeyGeneration(24-39)main(41-43)
indexer/decoder/decoder.go (2)
indexer/decoder/types.go (1)
BasicTxData(7-13)experiments/experiment6/main.go (1)
printAllData(27-44)
pkgs/database/types_api.go (1)
experiments/experiment6/main.go (1)
printAllData(27-44)
api/huma-types/transaction.go (2)
api/huma-types/address.go (1)
Body(15-17)api/handlers/transactions_test.go (1)
TestTransactionsHandler_GetTransactionBasic_Success(56-81)
indexer/query/types.go (5)
indexer/rpc_client/commit.go (1)
CommitResponse(10-15)indexer/rpc_client/errors.go (1)
RpcCommitError(36-40)indexer/rpc_client/blocks.go (2)
Type(49-58)br(136-141)indexer/rpc_client/rmc_test.go (1)
m(22-26)indexer/rpc_client/gen_data.go (1)
br(77-80)
integration/synthetic/init.go (2)
indexer/orchestrator/operator_test.go (1)
MockDatabaseHeight(97-100)api/handlers/blocks_test.go (2)
TestBlocksHandler_GetLatestBlockHeight_Fail(107-118)TestBlocksHandler_GetLatestBlockHeight_Success(96-105)
indexer/query/operator.go (4)
indexer/orchestrator/types.go (1)
QueryOperator(21-26)indexer/query/types.go (1)
QueryOperator(16-22)indexer/rpc_client/commit.go (1)
CommitResponse(10-15)indexer/retry/worker.go (1)
RetryWithContext(97-120)
indexer/rpc_client/client.go (3)
indexer/rpc_client/types.go (1)
RpcGnoland(10-13)indexer/rpc_client/commit.go (1)
CommitResponse(10-15)indexer/rpc_client/errors.go (1)
RpcCommitError(36-40)
indexer/rpc_client/commit.go (2)
indexer/rpc_client/types.go (1)
JsonRpcError(16-20)indexer/rpc_client/blocks.go (3)
BlockHeader(22-39)BlockID(10-13)Precommit(49-58)
pkgs/sql_data_types/table.go (3)
pkgs/database/types_api.go (1)
Amount(27-30)pkgs/sql_data_types/types.go (1)
Amount(8-11)pkgs/generator/general.go (1)
Amount(84-87)
indexer/orchestrator/types.go (3)
indexer/rpc_client/commit.go (1)
CommitResponse(10-15)indexer/rpc_client/blocks.go (1)
BlockResponse(86-91)indexer/rpc_client/transactions.go (1)
TxResponse(43-48)
indexer/query/operator_test.go (5)
indexer/rpc_client/blocks.go (1)
BlockResponse(86-91)indexer/rpc_client/errors.go (3)
RpcHeightError(6-10)RpcStringError(21-25)RpcCommitError(36-40)indexer/rpc_client/transactions.go (1)
TxResponse(43-48)indexer/rpc_client/commit.go (1)
CommitResponse(10-15)indexer/query/operator.go (1)
NewQueryOperator(20-46)
indexer/data_processor/operator.go (3)
pkgs/database/types_api.go (3)
MsgCall(42-53)MsgRun(68-79)AddressTx(98-102)indexer/rpc_client/commit.go (1)
CommitResponse(10-15)indexer/decoder/product.go (1)
DbMessageGroups(151-156)
indexer/data_processor/types.go (2)
pkgs/sql_data_types/table.go (8)
Blocks(101-108)ValidatorBlockSigning(139-150)TransactionGeneral(229-243)MsgSend(291-302)MsgCall(360-373)MsgAddPackage(427-441)MsgRun(495-509)AddressTx(182-188)pkgs/database/types_api.go (4)
MsgCall(42-53)MsgAddPackage(55-66)MsgRun(68-79)AddressTx(98-102)
pkgs/database/queries_indexer.go (1)
pkgs/database/types.go (1)
TimescaleDb(10-12)
api/handlers/interface.go (1)
pkgs/database/types_api.go (8)
AddressTx(98-102)BlockData(6-13)BlockSigners(92-96)Transaction(81-90)BankSend(32-40)MsgCall(42-53)MsgAddPackage(55-66)MsgRun(68-79)
pkgs/database/insert.go (3)
pkgs/database/types.go (1)
TimescaleDb(10-12)pkgs/sql_data_types/table.go (8)
Blocks(101-108)ValidatorBlockSigning(139-150)TransactionGeneral(229-243)AddressTx(182-188)MsgSend(291-302)MsgCall(360-373)MsgAddPackage(427-441)MsgRun(495-509)pkgs/database/types_api.go (4)
AddressTx(98-102)MsgCall(42-53)MsgAddPackage(55-66)MsgRun(68-79)
integration/synthetic/respone_maker.go (4)
pkgs/generator/general.go (1)
DataGenerator(37-41)indexer/rpc_client/blocks.go (3)
Precommit(49-58)BlockID(10-13)BlockHeader(22-39)indexer/rpc_client/transactions.go (1)
TxResponse(43-48)indexer/rpc_client/commit.go (4)
CommitResponse(10-15)CommitResult(18-20)SignedHeader(23-26)Commit(29-32)
indexer/orchestrator/operator_test.go (2)
indexer/rpc_client/commit.go (1)
CommitResponse(10-15)integration/synthetic/init.go (1)
MockDatabaseHeight(81-83)
api/handlers/database_test.go (1)
pkgs/database/types_api.go (8)
BlockData(6-13)BlockSigners(92-96)Transaction(81-90)AddressTx(98-102)BankSend(32-40)MsgCall(42-53)MsgAddPackage(55-66)MsgRun(68-79)
pkgs/database/queries_api.go (3)
pkgs/database/types.go (1)
TimescaleDb(10-12)pkgs/database/types_api.go (8)
BlockData(6-13)BlockSigners(92-96)BankSend(32-40)Amount(27-30)MsgCall(42-53)MsgAddPackage(55-66)MsgRun(68-79)Transaction(81-90)pkgs/sql_data_types/table.go (3)
MsgCall(360-373)MsgAddPackage(427-441)MsgRun(495-509)
indexer/orchestrator/operator.go (3)
indexer/orchestrator/types.go (1)
Orchestrator(50-60)indexer/rpc_client/blocks.go (1)
BlockResponse(86-91)indexer/rpc_client/commit.go (1)
CommitResponse(10-15)
integration/synthetic/query_operator.go (2)
indexer/rpc_client/commit.go (1)
CommitResponse(10-15)integration/synthetic/respone_maker.go (3)
ResponseMaker(11-13)NewResponseMaker(15-19)GenCommitInput(169-175)
🪛 checkmake (0.2.2)
Makefile
[warning] 1-1: Missing required phony target "all"
(minphony)
🪛 LanguageTool
CHANGELOG.md
[style] ~14-~14: Consider removing “of” to be more concise
Context: ...from the last commit, which is actually all of the block signers from the previous block. ...
(ALL_OF_THE)
[grammar] ~14-~14: Ensure spelling is correct
Context: ... signers from the previous block. So it woult insert it like it was meant for that bl...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~15-~15: Ensure spelling is correct
Context: ...hold this data properly and would cause and error because in the postgres primary k...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~15-~15: Ensure spelling is correct
Context: ...nd would cause and error because in the postgres primary key was attached making each me...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~21-~21: Ensure spelling is correct
Context: ...his is that indexer at that moment will fecth 2X times more request to the RPC, and i...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: integration-test-small
- GitHub Check: Analyze (go)
…n binary, removed expermintal-install option from the make file
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
indexer/data_processor/operator.go (2)
247-271: Potential data corruption: slice truncation doesn't account for sparse indices.When goroutines return early (lines 228, 234, 238, 244), they leave zero-values at their index positions. The slice is written at specific indices (
transactionsData[idx]), but truncation at line 271 assumesvalidCountvalid entries are contiguous at the start.Example: If transactions 0, 2, 4 succeed and 1, 3 fail,
validCount=3but valid data is at indices 0, 2, 4. Truncating to[:3]includes the zero-value at index 1.Consider using append with mutex instead:
- transactionsData := make([]sqlDataTypes.TransactionGeneral, transactionAmount) + transactionsData := make([]sqlDataTypes.TransactionGeneral, 0, transactionAmount) var mu sync.Mutex - var validCount int wg := sync.WaitGroup{} ... - transactionsData[idx] = sqlDataTypes.TransactionGeneral{...} - validCount++ + mu.Lock() + transactionsData = append(transactionsData, sqlDataTypes.TransactionGeneral{...}) mu.Unlock() ... - transactionsData = transactionsData[:validCount]
338-342: Message-to-transaction order mismatch due to unordered channel collection.Goroutines send decoded messages to
addressCollectionChanin non-deterministic order. When collected at lines 340-342,allDecodedMsgswon't maintain the same order astransactions. Later at line 409,allDecodedMsgs[idx]is paired withtransactions[idx], causing mismatched pairings.Consider using index-based assignment like
ProcessBlocks:- addressCollectionChan := make(chan []*decoder.DecodedMsg, transactionAmount) + allDecodedMsgs := make([]*decoder.DecodedMsg, transactionAmount) wg1 := sync.WaitGroup{} wg1.Add(transactionAmount) - for _, transaction := range transactions { - go func(transaction TrasnactionsData) { + for idx, transaction := range transactions { + go func(idx int, transaction TrasnactionsData) { defer wg1.Done() decodedMsg := decoder.NewDecodedMsg(transaction.Response.Result.Tx) if decodedMsg == nil { - addressCollectionChan <- []*decoder.DecodedMsg{nil} + // nil already at allDecodedMsgs[idx] return } // ... address collection ... - addressCollectionChan <- []*decoder.DecodedMsg{decodedMsg} - }(transaction) + allDecodedMsgs[idx] = decodedMsg + }(idx, transaction) } - // Remove channel closing goroutine and channel collection loop + wg1.Wait()
♻️ Duplicate comments (1)
CHANGELOG.md (1)
14-15: Fix remaining grammar and capitalization errors in release notes.Several issues from previous review feedback remain unfixed:
- Line 14: "all of the block signers" has redundant "of" (should be "all the block signers")
- Line 15: "postgres" should be capitalized as "Postgres" (product name)
- Line 21: "request" is singular but should be plural "requests"; also "this requests" should be "these requests"
- The indexer would go into the blocks data and store the signers from the last commit, which is actually all of the block signers from the previous block. So it would insert it like it was meant for that block height. From now on the indexer will fetch data from the /commit method and insert it properly. [de40740](...) + The indexer would go into the blocks data and store the signers from the last commit, which is actually all the block signers from the previous block. So it would insert it like it was meant for that block height. From now on the indexer will fetch data from the /commit method and insert it properly. [de40740](...) - Gnoland can indeed execute multiple message types in the same transaction. The indexer wasn't able to hold this data properly and would cause an error because in the postgres primary key was attached making each message type unique. Now each message type has a message_counter which is a smallint(int16) which is used as a index for that transaction. [d647901](...) + Gnoland can indeed execute multiple message types in the same transaction. The indexer wasn't able to hold this data properly and would cause an error because in the Postgres primary key was attached making each message type unique. Now each message type has a message_counter which is a smallint(int16) which is used as a index for that transaction. [d647901](...) - The Orchestrator needs to make a fetch to the commit RPC method. This operation is done side by side with fetching the blocks method. A side effect of this is that indexer at that moment will fetch 2 times more request to the RPC, and if the Gnoland node has a limit on the amount of RPC clients that can use it, it might cause the indexer to slow down and throw errors on this requests. In future releases this should be improved. [d647901](...) + The Orchestrator needs to make a fetch to the commit RPC method. This operation is done side by side with fetching the blocks method. A side effect of this is that indexer at that moment will fetch 2 times more requests to the RPC, and if the Gnoland node has a limit on the amount of RPC clients that can use it, it might cause the indexer to slow down and throw errors on these requests. In future releases this should be improved. [d647901](...)Also applies to: 21-21
🧹 Nitpick comments (5)
indexer/data_processor/operator.go (4)
62-98: LGTM! Consider batching lock operations for performance.The mutex-protected map pattern is correct. For slight performance improvement, you could collect addresses locally within each goroutine and lock once to merge them:
// Inside goroutine - collect locally first localAddrs := make([]string, 0, len(precommits)+1) for _, precommit := range precommits { if precommit != nil { localAddrs = append(localAddrs, precommit.ValidatorAddress) } } localAddrs = append(localAddrs, proposer) // Single lock to merge mu.Lock() for _, addr := range localAddrs { addressesMap[addr] = struct{}{} } mu.Unlock()This reduces lock contention in high-volume scenarios.
162-172: Mutex is unnecessary for index-based slice writes.Since each goroutine writes to a distinct index
blocksData[idx]in a preallocated slice, concurrent writes are safe without synchronization. The slice header isn't modified (no append), only element assignment.- // Use mutex only when writing to shared slice - mu.Lock() blocksData[idx] = sqlDataTypes.Blocks{ Hash: hash, Height: height, Timestamp: block.Result.Block.Header.Time, ChainID: block.Result.Block.Header.ChainID, Txs: txs, ChainName: d.chainName, } - mu.Unlock()You can also remove the
var mu sync.Mutexdeclaration on line 122.
580-584: LGTM! Minor: preferdefer cancel()for consistency.The commit-based processing logic is correct. For consistency with other functions (e.g., line 181), consider using
defer cancel():ctx, cancel := context.WithTimeout(context.Background(), timeout) + defer cancel() err := d.dbPool.InsertValidatorBlockSignings(ctx, validatorData) - cancel()
603-604: Consider preallocating theaddressesslice.The slice grows via append under mutex contention. Since you know
txAmount, you can estimate and preallocate:- var addresses []sqlDataTypes.AddressTx + // Estimate ~2 addresses per message on average + addresses := make([]sqlDataTypes.AddressTx, 0, txAmount*2)This reduces allocations and potential slice copying during concurrent appends.
Makefile (1)
1-6: Consider adding an "all" phony target for convenience.While not required, adding an
alltarget is a common Makefile convention that provides a default build action when runningmakewithout arguments. This typically builds all main artifacts.You could add this after line 6:
all: build build-apiAnd update the .PHONY declaration:
-.PHONY: build install clean build-experimental install-experimental build-api test-race integration-test test +.PHONY: all build install clean build-experimental build-api test-race integration-test test
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
CHANGELOG.md(1 hunks)Makefile(2 hunks)api/main.go(4 hunks)indexer/data_processor/operator.go(24 hunks)pkgs/generator/generator_test.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- pkgs/generator/generator_test.go
🧰 Additional context used
🧬 Code graph analysis (1)
indexer/data_processor/operator.go (2)
indexer/decoder/types.go (1)
DecodedMsg(34-37)indexer/decoder/product.go (1)
DbMessageGroups(151-156)
🪛 checkmake (0.2.2)
Makefile
[warning] 1-1: Missing required phony target "all"
(minphony)
🪛 LanguageTool
CHANGELOG.md
[style] ~14-~14: Consider removing “of” to be more concise
Context: ...from the last commit, which is actually all of the block signers from the previous block. ...
(ALL_OF_THE)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: integration-test-small
- GitHub Check: Analyze (go)
🔇 Additional comments (9)
indexer/data_processor/operator.go (1)
451-508: Timeout formula fixed - consistent pattern across all message types.The timeout calculation
10*time.Second + (time.Duration(count) * time.Second / 5)correctly adds 0.2 seconds per message, addressing the issue from the previous review. The pattern is now consistent with other insert operations.api/main.go (4)
4-4: LGTM! Standard embed pattern for serving static assets.The embed directive and imports are correctly structured for serving the favicon as an embedded asset.
Also applies to: 8-8, 22-23
111-128: LGTM! Comprehensive OpenAPI metadata setup.The dynamic OpenAPI info construction provides complete API documentation metadata including contact details, license, and external documentation links. Version normalization via
TrimPrefixensures consistent formatting.
151-160: LGTM! Previous critical issue has been addressed.The favicon handler now correctly handles write errors gracefully using
log.Printfandhttp.Errorinstead oflog.Fatalf, preventing the server from crashing on transient failures.
163-215: LGTM! Consistent per-route OpenAPI metadata.The operation callbacks provide clear summaries and descriptions for each endpoint, enhancing the API documentation quality in a consistent manner across all routes.
Makefile (4)
1-1: LGTM! .PHONY list correctly updated.The phony targets list now includes the new test targets and correctly removes
install-experimentalwhich was deleted from the Makefile.
11-12: LGTM! Enhanced version derivation for development builds.The addition of
GIT_BRANCHand updatedVERSIONlogic provides better version identification for untagged builds (e.g., "dev-a1b2c3d"), improving traceability during development.
37-44: LGTM! Well-structured test targets.The test targets follow Go best practices:
testruns all tests with verbose outputtest-raceenables the race detector for concurrency issuesintegration-testproperly isolates integration tests with build tags and includes a generous 20-minute timeout for long-running tests
16-16: File rename successfully completed — no issues found.The verification confirms that:
indexer/indexer.goexists withpackage mainandfunc main()at line 9indexer/main.gohas been properly removed- The build, install, and experimental build targets correctly reference the new entry point
The structural change in the Makefile is valid.
| // Phase 1: Concurrent address collection using sync.Map | ||
| var addressesMap sync.Map | ||
| addressCollectionChan := make(chan []*decoder.DecodedMsg, len(transactions)) | ||
| // Phase 1: Concurrent address collection using map[strinct]struct{} |
There was a problem hiding this comment.
Typo in comment.
map[strinct]struct{} should be map[string]struct{}.
🤖 Prompt for AI Agents
In indexer/data_processor/operator.go around line 302, the inline comment
contains a typo "map[strinct]struct{}" — update the comment to
"map[string]struct{}" so it accurately documents the data structure being used;
no code changes required beyond correcting the comment text.
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/integration-tests.yml(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: integration-test-small
- GitHub Check: Analyze (go)
🔇 Additional comments (1)
.github/workflows/integration-tests.yml (1)
2-4: LGTM: Permissions block follows least-privilege principle.Adding explicit read permissions for
contentsandpackagesis a security best practice for CI workflows.
| - name: Initialize database schema | ||
| run: | | ||
| go run indexer/main.go setup create-db --db-user postgres --db-name postgres | ||
| go run indexer/indexer.go setup create-db --db-user postgres --db-name postgres |
There was a problem hiding this comment.
Critical: Database schema mismatch in small test setup.
The setup command creates the schema in the postgres database (--db-name postgres), but the test config connects to the gnoland database (line 75). The schema won't exist in the target database, causing test failures.
The small test's container (line 33) initializes with POSTGRES_DB: postgres, but the test config should target postgres database, not gnoland. Alternatively, pass --db-name gnoland to the setup command.
Proposed fix (option 1: align setup with test config):
- go run indexer/indexer.go setup create-db --db-user postgres --db-name postgres
+ go run indexer/indexer.go setup create-db --db-user postgres --db-name gnolandOr (option 2: align test config with setup):
- dbname: gnoland
+ dbname: postgresWhich approach is correct depends on your data model. Please verify.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| go run indexer/indexer.go setup create-db --db-user postgres --db-name postgres | |
| go run indexer/indexer.go setup create-db --db-user postgres --db-name gnoland |
🤖 Prompt for AI Agents
In .github/workflows/integration-tests.yml around line 60, the setup command
creates the schema in the postgres database (--db-name postgres) while the test
config targets gnoland (line 75), causing a schema mismatch; fix by either (A)
changing the setup command to use --db-name gnoland so the schema is created in
the database the tests connect to, or (B) change the test configuration (and any
DB env like POSTGRES_DB) to use postgres so it matches the setup step—pick the
correct option for your data model and update the workflow lines accordingly so
setup and test DB names are identical.
| - name: Initialize database schema | ||
| run: | | ||
| go run indexer/main.go setup create-db | ||
| go run indexer/indexer.go setup create-db --db-user postgres --db-name postgres |
There was a problem hiding this comment.
Critical: Database schema mismatch in medium test setup.
Same issue as the small test: the setup command creates the schema in the postgres database (--db-name postgres), but the test config connects to gnoland (line 149). The schema won't exist in the target database.
Unlike the small test, the container (line 107) initializes with POSTGRES_DB: gnoland, suggesting the intended target is gnoland. Pass --db-name gnoland to the setup command:
- go run indexer/indexer.go setup create-db --db-user postgres --db-name postgres
+ go run indexer/indexer.go setup create-db --db-user postgres --db-name gnoland📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| go run indexer/indexer.go setup create-db --db-user postgres --db-name postgres | |
| go run indexer/indexer.go setup create-db --db-user postgres --db-name gnoland |
🤖 Prompt for AI Agents
.github/workflows/integration-tests.yml around line 134: the setup step runs the
indexer with --db-name postgres while the Postgres container is initialized with
POSTGRES_DB: gnoland and the test config connects to gnoland, causing a schema
mismatch. Update the setup command to pass --db-name gnoland (so the schema is
created in the same database the tests connect to) and verify any other DB-name
references in this workflow match gnoland.
| - name: Initialize database schema | ||
| run: | | ||
| go run indexer/main.go setup create-db | ||
| go run indexer/indexer.go setup create-db |
There was a problem hiding this comment.
🧩 Analysis chain
Script executed:
#!/bin/bash
# Examine the workflow file around the relevant lines
cat -n .github/workflows/integration-tests.yml | sed -n '55,65p; 130,140p; 205,215p'Repository: Cogwheel-Validator/spectra-gnoland-indexer
<shell_result>
Repository: Cogwheel-Validator/spectra-gnoland-indexer
Exit code: 0
stdout:
55 env:
56 PGPASSWORD: test_password_12345678
57
58 - name: Initialize database schema
59 run: |
60 go run indexer/indexer.go setup create-db --db-user postgres --db-name postgres
61 env:
62 DB_HOST: localhost
63 DB_PORT: 5432
64 DB_USER: gnoland
65 DB_PASSWORD: test_password_12345678
130 PGPASSWORD: test_password_12345678
131
132 - name: Initialize database schema
133 run: |
134 go run indexer/indexer.go setup create-db --db-user postgres --db-name postgres
135 env:
136 DB_HOST: localhost
137 DB_PORT: 5432
138 DB_USER: postgres
139 DB_PASSWORD: test_password_12345678
140 DB_NAME: gnoland
205
206 - name: Initialize database schema
207 run: |
208 go run indexer/indexer.go setup create-db
209 env:
210 DB_HOST: localhost
211 DB_PORT: 5432
212 DB_USER: postgres
213 DB_PASSWORD: test_password_12345678
214 DB_NAME: gnoland
215
</shell_result>
Script executed:
#!/bin/bash
# Find indexer.go and examine its setup and create-db command handling
find . -name "indexer.go" -type f | head -5Repository: Cogwheel-Validator/spectra-gnoland-indexer
<shell_result>
Repository: Cogwheel-Validator/spectra-gnoland-indexer
Exit code: 0
stdout:
./indexer/indexer.go
</shell_result>
Script executed:
#!/bin/bash
# Check file size then examine setup create-db command handling
wc -l ./indexer/indexer.goRepository: Cogwheel-Validator/spectra-gnoland-indexer
<shell_result>
Repository: Cogwheel-Validator/spectra-gnoland-indexer
Exit code: 0
stdout:
13 ./indexer/indexer.go
</shell_result>
Script executed:
#!/bin/bash
# Read the full indexer.go file and search for setup command implementation
cat -n ./indexer/indexer.go && find ./indexer -name "*.go" -type f | grep -E "(setup|cmd)" | head -10Repository: Cogwheel-Validator/spectra-gnoland-indexer
<shell_result>
Repository: Cogwheel-Validator/spectra-gnoland-indexer
Exit code: 0
stdout:
1 package main
2
3 import (
4 "log"
5
6 "github.com/Cogwheel-Validator/spectra-gnoland-indexer/indexer/cmd"
7 )
8
9 func main() {
10 if err := cmd.RootCmd.Execute(); err != nil {
11 log.Fatalf("failed to execute command: %v", err)
12 }
13 }
./indexer/cmd/func.go
./indexer/cmd/historic.go
./indexer/cmd/live.go
./indexer/cmd/root.go
./indexer/cmd/run.go
./indexer/cmd/setup.go
</shell_result>
Script executed:
#!/bin/bash
# Check setup.go size and examine it
wc -l ./indexer/cmd/setup.go
cat -n ./indexer/cmd/setup.goRepository: Cogwheel-Validator/spectra-gnoland-indexer
<shell_result>
Repository: Cogwheel-Validator/spectra-gnoland-indexer
Exit code: 0
stdout:
272 ./indexer/cmd/setup.go
1 package cmd
2
3 import (
4 "context"
5 "log"
6 "time"
7
8 dbinit "github.com/Cogwheel-Validator/spectra-gnoland-indexer/indexer/db_init"
9 "github.com/Cogwheel-Validator/spectra-gnoland-indexer/pkgs/database"
10 "github.com/Cogwheel-Validator/spectra-gnoland-indexer/pkgs/sql_data_types"
11 "github.com/spf13/cobra"
12 )
13
14 var allowedSslModes = []string{"disable", "require", "verify-ca", "verify-full", "allow", "prefer"}
15
16 // dbParams holds common database connection parameters
17 type dbParams struct {
18 host string
19 port int
20 user string
21 name string
22 password string
23 sslMode string
24 }
25
26 var setupCmd = &cobra.Command{
27 Use: "setup",
28 Short: "Database setup tools",
29 Long: `A collection of tools to set up and manage the database for the gnoland indexer.`,
30 }
31
32 func init() {
33 // Add subcommands
34 setupCmd.AddCommand(createDbCmd)
35 setupCmd.AddCommand(createUserCmd)
36 setupCmd.AddCommand(createConfigCmd)
37
38 // Common flags for both database setup commands
39 for _, cmd := range []*cobra.Command{createDbCmd, createUserCmd} {
40 cmd.Flags().StringP("db-host", "b", "", "The database host, default is localhost")
41 cmd.Flags().IntP("db-port", "p", 0, "The database port, default is 5432")
42 cmd.Flags().StringP("db-user", "u", "", "The database user, default is postgres")
43 cmd.Flags().StringP("db-name", "d", "", "The database name, default is postgres")
44 cmd.Flags().StringP("ssl-mode", "s", "", "The SSL mode for the database connection, default is disable")
45 }
46
47 // create-user specific flags
48 createUserCmd.Flags().StringP("privilege", "r", "", "The privilege level for the user (reader or writer)")
49 createUserCmd.Flags().String("user", "", "The user name for the user to create")
50
51 // create-db specific flags
52 createDbCmd.Flags().String("new-db-name", "", "The database name to create, default is gnoland")
53 createDbCmd.Flags().String("chain-name", "", "The chain name for the database type enum, default is gnoland")
54
55 // create-config specific flags
56 createConfigCmd.Flags().StringP("config", "c", "config.yaml", "The config file name, default is config.yaml")
57 createConfigCmd.Flags().BoolP("overwrite", "o", false, "Overwrite the existing config file, default is false")
58 }
59
60 var createDbCmd = &cobra.Command{
61 Use: "create-db",
62 Short: "Create a new database named gnoland",
63 Long: `Create a new database named gnoland for the indexer. It goes\n
64 throught a lot of steps to create the database and insert the tables and data.`,
65 RunE: func(cmd *cobra.Command, args []string) error {
66 log.Printf("Initiating the cmd to set up the database for the indexer...")
67
68 // Parse and validate common database flags
69 params, err := parseCommonFlags(cmd, "postgres")
70 if err != nil {
71 log.Fatalf("failed to parse flags: %v", err)
72 }
73
74 // get the new database name from the flags
75 newDbName, _ := cmd.Flags().GetString("new-db-name")
76 if newDbName == "" {
77 newDbName = "gnoland"
78 }
79
80 // get the chain name from the flags
81 chainName, _ := cmd.Flags().GetString("chain-name")
82 if chainName == "" {
83 chainName = "gnoland"
84 }
85
86 // Prompt for password
87 params.password, err = promptPassword()
88 if err != nil {
89 log.Fatalf("failed to read password: %v", err)
90 }
91
92 // Create database config
93 dbConfig := params.createDatabaseConfig()
94
95 // create a new database connection
96 db := database.NewTimescaleDbSetup(dbConfig)
97
98 // create a new database named "gnoland"
99 // but check if the current database is "gnoland"
100 ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
101 defer cancel()
102 currentDb, err := db.CheckCurrentDatabaseName(ctx)
103 if err != nil {
104 log.Fatalf("failed to check current database name: %v", err)
105 }
106 log.Printf("Logged in into the database %s", currentDb)
107
108 // if the current database is not "gnoland", create a new database named "gnoland"
109 // and insert all of the tables and data from the "gnoland" database
110 if currentDb != newDbName {
111 // create a new database named "gnoland"
112 log.Printf("Creating a new database named %s", newDbName)
113 err = database.CreateDatabase(db, newDbName)
114 if err != nil {
115 log.Fatalf("failed to create database: %v", err)
116 }
117 // switch to the new database
118 log.Printf("Switching to the new database %s", newDbName)
119 // only for now later add dbName value, it is only for the testing now
120 err = database.SwitchDatabase(db, dbConfig, newDbName)
121 if err != nil {
122 log.Fatalf("failed to switch database: %v", err)
123 }
124 // insert all of the tables and data from the new database
125 // First create special types (custom postgres types that tables depend on)
126 // and type enums
127 specialTypes := []sql_data_types.DBSpecialType{
128 sql_data_types.Amount{},
129 sql_data_types.Attribute{}, // this needs to be inserted prior to event type
130 sql_data_types.Event{},
131 }
132 typeEnums := []string{
133 chainName,
134 }
135
136 // Initialize database initializer
137 dbInit := dbinit.NewDBInitializer(db.GetPool())
138
139 // Create special types first (they need to exist before tables that use them)
140 log.Printf("Inserting all of the special types into the %s database", chainName)
141 for _, specialType := range specialTypes {
142 err = dbInit.CreateSpecialTypeFromStruct(specialType, specialType.TypeName())
143 if err != nil {
144 log.Fatalf("failed to create special type %s: %v", specialType.TypeName(), err)
145 }
146 }
147
148 // Create type enums
149 log.Printf("Inserting all of the type enums into the %s database", chainName)
150 err = dbInit.CreateChainTypeEnum(typeEnums)
151 if err != nil {
152 log.Fatalf("failed to create type enum %s: %v", typeEnums, err)
153 }
154
155 // Create regular tables (non-time-series tables)
156 log.Printf("Inserting all of the regular tables into the %s database", chainName)
157 regularTables := []sql_data_types.DBTable{
158 sql_data_types.GnoAddress{},
159 sql_data_types.GnoValidatorAddress{},
160 }
161
162 for _, dataType := range regularTables {
163 err = dbInit.CreateTableFromStruct(dataType, dataType.TableName())
164 if err != nil {
165 log.Fatalf("failed to create table %s: %v", dataType.TableName(), err)
166 }
167 }
168
169 // Create hypertables (time-series tables with timestamp columns)
170 log.Printf("Inserting all of the hypertables into the %s database", chainName)
171 hypertables := []struct {
172 table sql_data_types.DBTable
173 partitionColumn string
174 chunkInterval string
175 }{
176 {sql_data_types.Blocks{}, "timestamp", "1 week"},
177 {sql_data_types.ValidatorBlockSigning{}, "timestamp", "1 week"},
178 {sql_data_types.AddressTx{}, "timestamp", "1 week"},
179 {sql_data_types.TransactionGeneral{}, "timestamp", "1 week"},
180 {sql_data_types.MsgSend{}, "timestamp", "1 week"},
181 {sql_data_types.MsgCall{}, "timestamp", "1 week"},
182 {sql_data_types.MsgAddPackage{}, "timestamp", "1 week"},
183 {sql_data_types.MsgRun{}, "timestamp", "1 week"},
184 }
185
186 for _, ht := range hypertables {
187 err = dbInit.CreateHypertableFromStruct(ht.table, ht.table.TableName(), ht.partitionColumn, ht.chunkInterval)
188 if err != nil {
189 log.Fatalf("failed to create hypertable %s: %v", ht.table.TableName(), err)
190 }
191 }
192 log.Printf("Successfully created all of the hypertables into the %s database", chainName)
193 } else {
194 log.Printf("The current database is %s, and it already exists", currentDb)
195 // TODO else if the current database is "gnoland" then we need to check if the tables exist
196 // and if they don't exist then we need to create them
197 // also any kind of future updates to the database should be done here
198 }
199 return nil
200 },
201 }
202
203 var createUserCmd = &cobra.Command{
204 Use: "create-user",
205 Short: "Create a new user for the database",
206 Long: `Create a new user for the database. It will ask for the password and create the user.`,
207 RunE: func(cmd *cobra.Command, args []string) error {
208 // Parse and validate common database flags
209 params, err := parseCommonFlags(cmd, "gnoland")
210 if err != nil {
211 log.Fatalf("failed to parse flags: %v", err)
212 }
213
214 // Get privilege flag
215 privilege, _ := cmd.Flags().GetString("privilege")
216 if privilege == "" {
217 log.Fatalf("privilege is required")
218 } else if privilege != "reader" && privilege != "writer" {
219 log.Fatalf("invalid privilege: %s", privilege)
220 }
221
222 // get the user name from the flags
223 userName, _ := cmd.Flags().GetString("user")
224 if userName == "" {
225 log.Fatalf("user name is required")
226 }
227
228 // Prompt for password
229 params.password, err = promptPassword()
230 if err != nil {
231 log.Fatalf("failed to read password: %v", err)
232 }
233
234 // Create database config and connection
235 dbConfig := params.createDatabaseConfig()
236 db := database.NewTimescaleDbSetup(dbConfig)
237 dbInit := dbinit.NewDBInitializer(db.GetPool())
238
239 // Create a new user
240 err = dbInit.CreateUser(userName)
241 if err != nil {
242 log.Fatalf("failed to create user: %v", err)
243 }
244
245 // Appoint privileges to the user
246 err = dbInit.AppointPrivileges(params.user, privilege, []string{})
247 if err != nil {
248 log.Fatalf("failed to appoint privileges to user: %v", err)
249 }
250
251 log.Printf("Successfully created user %s with privilege %s", params.user, privilege)
252 return nil
253 },
254 }
255
256 var createConfigCmd = &cobra.Command{
257 Use: "create-config",
258 Short: "Generate a config with default values.",
259 Long: `Generate a config with default values. It will make a config file with default values.
260 You can add --overwrite to overwrite the existing config file. And you can use --config to specifly the path`,
261 RunE: func(cmd *cobra.Command, args []string) error {
262 // get the config file name from the flags
263 configFileName, _ := cmd.Flags().GetString("config")
264 if configFileName == "" {
265 configFileName = "config.yaml"
266 }
267 overwrite, _ := cmd.Flags().GetBool("overwrite")
268 createConfig(overwrite, configFileName)
269 log.Printf("Successfully created config file %s", configFileName)
270 return nil
271 },
272 }
</shell_result>
Script executed:
#!/bin/bash
# Find parseCommonFlags function definition
grep -n "func parseCommonFlags" ./indexer/cmd/*.goRepository: Cogwheel-Validator/spectra-gnoland-indexer
<shell_result>
Repository: Cogwheel-Validator/spectra-gnoland-indexer
Exit code: 0
stdout:
./indexer/cmd/func.go:20:func parseCommonFlags(cmd *cobra.Command, defaultDbName string) (*dbParams, error) {
</shell_result>
Script executed:
#!/bin/bash
# Read func.go to understand parseCommonFlags and env var handling
cat -n ./indexer/cmd/func.goRepository: Cogwheel-Validator/spectra-gnoland-indexer
<shell_result>
Repository: Cogwheel-Validator/spectra-gnoland-indexer
Exit code: 0
stdout:
1 package cmd
2
3 import (
4 "fmt"
5 "log"
6 "os"
7 "path/filepath"
8 "slices"
9 "syscall"
10 "time"
11
12 "github.com/Cogwheel-Validator/spectra-gnoland-indexer/indexer/config"
13 "github.com/Cogwheel-Validator/spectra-gnoland-indexer/pkgs/database"
14 "github.com/spf13/cobra"
15 "go.yaml.in/yaml/v4"
16 "golang.org/x/term"
17 )
18
19 // parseCommonFlags extracts and validates common database flags
20 func parseCommonFlags(cmd *cobra.Command, defaultDbName string) (*dbParams, error) {
21 params := &dbParams{}
22
23 params.host, _ = cmd.Flags().GetString("db-host")
24 params.port, _ = cmd.Flags().GetInt("db-port")
25 params.user, _ = cmd.Flags().GetString("db-user")
26 params.sslMode, _ = cmd.Flags().GetString("ssl-mode")
27 params.name, _ = cmd.Flags().GetString("db-name")
28
29 // Apply environment variable fallbacks (for CI/CD)
30 if params.host == "" {
31 if envHost := os.Getenv("DB_HOST"); envHost != "" {
32 params.host = envHost
33 }
34 }
35 if params.port == 0 {
36 if envPort := os.Getenv("DB_PORT"); envPort != "" {
37 if _, err := fmt.Sscanf(envPort, "%d", ¶ms.port); err != nil {
38 return nil, fmt.Errorf("failed to scan port: %v", err)
39 }
40 }
41 }
42 if params.user == "" {
43 if envUser := os.Getenv("DB_USER"); envUser != "" {
44 params.user = envUser
45 }
46 }
47 if params.name == "" {
48 if envDbName := os.Getenv("DB_NAME"); envDbName != "" {
49 params.name = envDbName
50 }
51 }
52
53 // Apply defaults if still empty
54 if params.sslMode == "" {
55 params.sslMode = "disable"
56 }
57 if params.host == "" {
58 params.host = "localhost"
59 }
60 if params.port == 0 {
61 params.port = 5432
62 }
63 if params.user == "" {
64 params.user = "postgres"
65 }
66 if params.name == "" {
67 params.name = defaultDbName
68 }
69
70 // Validate
71 if !slices.Contains(allowedSslModes, params.sslMode) {
72 return nil, fmt.Errorf("invalid ssl mode: %s", params.sslMode)
73 }
74 if params.port < 1 || params.port > 65535 {
75 return nil, fmt.Errorf("invalid port: %d", params.port)
76 }
77
78 return params, nil
79 }
80
81 // promptPassword prompts user for password input or reads from environment
82 func promptPassword() (string, error) {
83 // First check if password is provided via environment variable (for CI/CD)
84 if envPassword := os.Getenv("DB_PASSWORD"); envPassword != "" {
85 return envPassword, nil
86 }
87
88 // Interactive mode: prompt user for password
89 fmt.Print("Enter the database password: ")
90 bytePassword, err := term.ReadPassword(int(syscall.Stdin))
91 if err != nil {
92 return "", fmt.Errorf("failed to read password: %v", err)
93 }
94 fmt.Println()
95 return string(bytePassword), nil
96 }
97
98 // createDatabaseConfig creates a DatabasePoolConfig from dbParams
99 func (p *dbParams) createDatabaseConfig() database.DatabasePoolConfig {
100 return database.DatabasePoolConfig{
101 Host: p.host,
102 Port: p.port,
103 User: p.user,
104 Dbname: p.name,
105 Password: p.password,
106 Sslmode: p.sslMode,
107 PoolMaxConns: 10,
108 PoolMinConns: 1,
109 PoolMaxConnLifetime: 10 * time.Minute,
110 PoolMaxConnIdleTime: 5 * time.Minute,
111 PoolHealthCheckPeriod: 1 * time.Minute,
112 PoolMaxConnLifetimeJitter: 1 * time.Minute,
113 }
114 }
115
116 func createConfig(overwrite bool, fileName string) {
117 if fileName == "" {
118 fileName = "config.yml"
119 }
120 absolutePath, err := filepath.Abs(fileName)
121 if err != nil {
122 log.Fatalf("failed to get absolute path: %v", err)
123 }
124 fileName = absolutePath
125 // config file
126 config := config.Config{
127 RpcUrl: "http://localhost:26657",
128 PoolMaxConns: 50,
129 PoolMinConns: 10,
130 PoolMaxConnLifetime: 5 * time.Minute,
131 PoolMaxConnIdleTime: 5 * time.Minute,
132 PoolHealthCheckPeriod: 1 * time.Minute,
133 PoolMaxConnLifetimeJitter: 1 * time.Minute,
134 LivePooling: 5 * time.Second,
135 MaxBlockChunkSize: 50,
136 MaxTransactionChunkSize: 100,
137 ChainName: "gnoland",
138 RetryAmount: &[]int{6}[0],
139 Pause: &[]int{3}[0],
140 PauseTime: &[]time.Duration{15 * time.Second}[0],
141 ExponentialBackoff: &[]time.Duration{2 * time.Second}[0],
142 }
143 // marshal the config to yaml
144 yamlFile, err := yaml.Marshal(config)
145 if err != nil {
146 log.Fatalf("failed to marshal config: %v", err)
147 }
148 // check if the config file exists
149 if _, err = os.Stat(fileName); err == nil {
150 if overwrite {
151 if writeErr := os.WriteFile(fileName, yamlFile, 0644); writeErr != nil {
152 log.Fatalf("failed to overwrite config file: %v", writeErr)
153 }
154 } else {
155 log.Fatalf("config file already exists, use --overwrite to overwrite it")
156 }
157 } else if os.IsNotExist(err) {
158 if writeErr := os.WriteFile(fileName, yamlFile, 0644); writeErr != nil {
159 log.Fatalf("failed to create config file: %v", writeErr)
160 }
161 } else {
162 log.Fatalf("failed to stat config file: %v", err)
163 }
164 }
</shell_result>
Script executed:
#!/bin/bash
# Verify the default db name logic for create-db
sed -n '65,78p' ./indexer/cmd/setup.goRepository: Cogwheel-Validator/spectra-gnoland-indexer
<shell_result>
Repository: Cogwheel-Validator/spectra-gnoland-indexer
Exit code: 0
stdout:
RunE: func(cmd *cobra.Command, args []string) error {
log.Printf("Initiating the cmd to set up the database for the indexer...")
// Parse and validate common database flags
params, err := parseCommonFlags(cmd, "postgres")
if err != nil {
log.Fatalf("failed to parse flags: %v", err)
}
// get the new database name from the flags
newDbName, _ := cmd.Flags().GetString("new-db-name")
if newDbName == "" {
newDbName = "gnoland"
}
</shell_result>
Based on my verification, I have confirmed the inconsistency claim. Here's what I found:
Verification Summary:
The parseCommonFlags function implements a clear precedence: command-line flags → environment variables → defaults.
-
Small/Medium tests (lines 60, 134): Explicitly pass
--db-user postgres --db-name postgres, which forces connections to the "postgres" database regardless of environment variables. -
Large test (line 208): Omits these flags and relies on
DB_NAME=gnoland(line 214), causing it to connect to the "gnoland" database instead.
This creates functionally different behavior:
- Small/Medium: Connect to "postgres" DB, then create "gnoland"
- Large: Connect to "gnoland" DB (from env var), then attempt creation (which is skipped if DB already exists due to line 110 check in setup.go)
The inconsistency is real and impacts initialization behavior across test sizes.
Inconsistency: Large test omits --db-user and --db-name flags, causing different connection database.
The small and medium tests explicitly pass --db-user postgres --db-name postgres (lines 60, 134), connecting to the "postgres" database for initialization. The large test omits these flags (line 208) and instead relies on the DB_NAME=gnoland environment variable (line 214), connecting to "gnoland" instead.
This creates inconsistent initialization behavior: small/medium tests connect to "postgres" to create/initialize "gnoland", while the large test connects directly to "gnoland". If the large test intends to initialize from "postgres" like the other tests, add the explicit flags. If "gnoland" is the intended starting database for large tests, document this intentional difference and verify the schema initialization handles both scenarios correctly.
🤖 Prompt for AI Agents
.github/workflows/integration-tests.yml around line 208: the large test run
omits the explicit db flags and relies on DB_NAME=gnoland which causes it to
connect to a different database than the small/medium tests; to fix, make the
large test consistent with the others by adding the same flags used in
small/medium (e.g. append --db-user postgres --db-name postgres to the go run
indexer/indexer.go setup create-db invocation) or alternatively set
DB_NAME=postgres for that step and ensure the env/flag precedence is documented;
choose the option that matches intended behavior and keep all three test sizes
consistent.
1 participant
Summary by CodeRabbit
New Features
Changes
Tests
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.